[SUREFIRE-2252] - Fix JUnit5 reporting when tests are executed in parallel #772
Conversation
|
There are test failures, please take a look. |
Looks like the POM issue I was referring to in the JIRA, on what value should I add to test my current logic/version. |
Will try to use |
ArvindJoshi-okta
force-pushed
the
SUREFIRE-2252
branch
from
5c627f9
to
809aeed
Compare
@michael-o Could you pls review now? All tests are passing. |
...latform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java
Outdated
Show resolved
Hide resolved
...it-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
Outdated
Show resolved
Hide resolved
ArvindJoshi-okta
force-pushed
the
SUREFIRE-2252
branch
from
809aeed
to
767e1a9
Compare
| launcher.execute(builder.build(), adapter); | ||
| } else { |
There was a problem hiding this comment.
How this has been collapsed and both cases are identical. Can you explain why this new code is correct for both? I honest cannot assess whether this is wrong or correct. What should happen in your opinion in the eager case?
There was a problem hiding this comment.
Thinking about this a bit, I think I am misunderstanding what the eager reading actually means. Could you please clarify its intention? How does an end user set it and where?
In my case here, what is happening is, in a parallel run, the test runs are combined due to eager reading, the classes get executed together and hence tests get reported as a bundle (I'll look into the reporting aspect, maybe that's the issue and not the execution). But in general, I felt there is no need for eager reading for a parallel run. Maybe we need to separate the 2 conditions?
There was a problem hiding this comment.
Thinking about this a bit, I think I am misunderstanding what the eager reading actually means. Could you please clarify its intention? How does an end user set it and where?
Unfortunately, I cannot answer these question because I have no idea.
In my case here, what is happening is, in a parallel run, the test runs are combined due to eager reading, the classes get executed together and hence tests get reported as a bundle (I'll look into the reporting aspect, maybe that's the issue and not the execution). But in general, I felt there is no need for eager reading for a parallel run. Maybe we need to separate the 2 conditions?
Is this similar: https://issues.apache.org/jira/browse/SUREFIRE-2195?
There was a problem hiding this comment.
Yes, 2195 seems like the same issue
There was a problem hiding this comment.
I honestly don't know how to proceed because I do not understand the implications :-(
There was a problem hiding this comment.
With this change I see in logs:
[INFO] Running pkg.domain.BxTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.014 s -- in pkg.domain.BxTest
[INFO] pkg.domain.BxTest.test -- Time elapsed: 0.007 s
[INFO] Running pkg.domain.AxTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 s -- in pkg.domain.AxTest
[INFO] pkg.domain.AxTest.test -- Time elapsed: 0 s
but without:
[INFO] Running pkg.domain.AxTest
[INFO] Running pkg.domain.BxTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 s -- in pkg.domain.AxTest
[INFO] pkg.domain.BxTest.test -- Time elapsed: 0.007 s
[INFO] pkg.domain.AxTest.test -- Time elapsed: 0.007 s
[INFO] Tests run: 0, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.015 s -- in pkg.domain.BxTest
so look like tests are executed sequentially not in parallel
So for me problem is something else ...
There was a problem hiding this comment.
The issue seems to be in the test listener which isn't parallel safe and is expecting results to trickle in sequentially, unsure of what needs to be fixed.
There was a problem hiding this comment.
I linked some of similar issues ... the problem is probbably in StatelessXmlReporter
There was a problem hiding this comment.
@slawekjaranowski Do you think you can pick up, I have honestly no clue here.
There was a problem hiding this comment.
One alternate approach I am investigating is to ignore these xmls and use Allure reports instead. They generate a bunch of json files which seem accurate and have all the relevant information.
|
How close is this to being merged? It is incredibly useful to us.. |
Merging is simple with a button, but understanding that this is right is hard. For everything else there is Mastercard. |
Problem
Fix
JUnitPlatformProviderwhere theexecutemethod was sending in all the classes at the same time in class selector list, hence combining the resultsChecklist
Following this checklist to help us incorporate your
contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
[SUREFIRE-XXX] - Fixes bug in ApproximateQuantiles,where you replace
SUREFIRE-XXXwith the appropriate JIRA issue. Best practiceis to use the JIRA issue title in the pull request title and in the first line of the
commit message.
mvn clean installto make sure basic checks pass. A more thorough check willbe performed on your pull request automatically.
mvn -Prun-its clean install).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004
In any other case, please file an Apache Individual Contributor License Agreement.